Refactor ORM Layer and Improve Docker Configuration#31
Merged
Frankoul merged 4 commits intoembyplus:mainfrom Mar 20, 2025
akaBoyLovesToCode:sqlalchemy
Merged
Refactor ORM Layer and Improve Docker Configuration#31Frankoul merged 4 commits intoembyplus:mainfrom akaBoyLovesToCode:sqlalchemy
Frankoul merged 4 commits intoembyplus:mainfrom
akaBoyLovesToCode:sqlalchemy
Conversation
…emy implementation - Create new database.py module with SQLAlchemy async pattern - Replace DBManager with DbOperations class for CRUD operations - Replace BaseOrmTable with SQLAlchemy declarative_base - Replace BaseOrmTableWithTS with timestamp columns implementation - Update application entry point with new database initialization flow - Refactor user_service.py to use repository pattern instead of ORM classes - Fix transaction handling with proper session management - Replace bulk operations with individual repository methods - Add proper error handling and logging in transactions - Update model updates to use repository methods instead of direct attribute changes - Implement get_session generator for session management - Improve code organization by moving repository code from app.py to model files
…bility - Change FROM python:3.12-alpine to python:3.12-slim - Maintain dependency installation through uv package manager - Preserve environment variables and docker configuration - Keep the application entrypoint using uv for running Python code This change addresses potential compatibility issues with Alpine Linux while maintaining a relatively small image footprint with the slim variant.
…upport - Replace alpine base image with python:3.12-slim for better compatibility - Fix PATH configuration to make uv available without source command - Add necessary development packages (libc6-dev, python3-dev) for C extensions - Use bash explicitly to support shell operations in build process - Improve dependency installation process using uv package manager - Organize package installation list for better readability - Ensure proper cleanup of build dependencies after installation - Remove redundant uv cache clean command to simplify build process - Maintain environment variables and Docker configuration This change resolves compilation errors with tgcrypto and other Python packages requiring C extensions, while keeping the Docker image as lightweight as possible.
User 模型更新后的类图classDiagram
class User {
- telegram_id: int
- is_admin: bool
- telegram_name: str
- emby_id: str
- emby_name: str
- enable_register: bool
- is_whitelist: bool
- ban_time: int
- reason: str
+ check_create_invite_code(): bool
+ check_create_whitelist_code(): bool
+ check_use_redeem_code(): bool
+ check_use_whitelist_code(): bool
+ is_emby_baned(): bool
+ emby_ban_info(): tuple[int, str]
}
class BaseModelWithTS {
- created_at: DateTime
- updated_at: DateTime
}
User --|> BaseModelWithTS : extends
note for User "使用 UserRepository 类替换 UserOrm 类以进行数据访问"
Config 模型更新后的类图classDiagram
class Config {
- total_register_user: int
- register_public_user: int
- register_public_time: int
}
class BaseModelWithTS {
- created_at: DateTime
- updated_at: DateTime
}
Config --|> BaseModelWithTS : extends
note for Config "使用 ConfigRepository 类替换 ConfigOrm 类以进行数据访问"
InviteCode 模型更新后的类图classDiagram
class InviteCode {
- code: str
- telegram_id: int
- code_type: Enum
- is_used: bool
- used_time: int
- used_user_id: int
}
class BaseModelWithTS {
- created_at: DateTime
- updated_at: DateTime
}
InviteCode --|> BaseModelWithTS : extends
note for InviteCode "使用 InviteCodeRepository 类替换 InviteCodeOrm 类以进行数据访问"
文件级别变更
提示和命令与 Sourcery 互动
自定义您的体验访问您的 仪表板 以：
获得帮助Original review guide in EnglishReviewer's Guide by SourceryThis pull request refactors the ORM layer to use native SQLAlchemy, improves the Docker configuration for better build reliability and dependency management, and removes the Sequence diagram for creating a new usersequenceDiagram
participant US as UserService
participant UR as UserRepository
participant EAPI as EmbyApi
participant CR as ConfigRepository
US->>UR: get_by_telegram_id(telegram_id)
alt User exists
UR-->>US: User
else User does not exist
US->>UR: create_user(telegram_id, is_admin, telegram_name)
UR-->>US: User
end
Updated class diagram for User modelclassDiagram
class User {
- telegram_id: int
- is_admin: bool
- telegram_name: str
- emby_id: str
- emby_name: str
- enable_register: bool
- is_whitelist: bool
- ban_time: int
- reason: str
+ check_create_invite_code(): bool
+ check_create_whitelist_code(): bool
+ check_use_redeem_code(): bool
+ check_use_whitelist_code(): bool
+ is_emby_baned(): bool
+ emby_ban_info(): tuple[int, str]
}
class BaseModelWithTS {
- created_at: DateTime
- updated_at: DateTime
}
User --|> BaseModelWithTS : extends
note for User "Replaces UserOrm class with UserRepository class for data access"
Updated class diagram for Config modelclassDiagram
class Config {
- total_register_user: int
- register_public_user: int
- register_public_time: int
}
class BaseModelWithTS {
- created_at: DateTime
- updated_at: DateTime
}
Config --|> BaseModelWithTS : extends
note for Config "Replaces ConfigOrm class with ConfigRepository class for data access"
Updated class diagram for InviteCode modelclassDiagram
class InviteCode {
- code: str
- telegram_id: int
- code_type: Enum
- is_used: bool
- used_time: int
- used_user_id: int
}
class BaseModelWithTS {
- created_at: DateTime
- updated_at: DateTime
}
InviteCode --|> BaseModelWithTS : extends
note for InviteCode "Replaces InviteCodeOrm class with InviteCodeRepository class for data access"
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey @Qubbby - I've reviewed your changes - here's some feedback:
Overall Comments:
- Consider adding a health check endpoint to the Dockerfile for improved container orchestration.
- The database initialization logic in
app.pyandmodels/database.pyis duplicated; consolidate it into a single function.
Here's what I looked at during the review
- 🟡 General issues: 1 issue found
- 🟢 Security: all looks good
- 🟢 Testing: all looks good
- 🟡 Complexity: 1 issue found
- 🟢 Documentation: all looks good
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
|
|
||
| class ConfigOrm(DBManager): | ||
| orm_table = Config | ||
| class ConfigRepository: |
There was a problem hiding this comment.
issue: Duplicate invite code repository methods in ConfigRepository.
ConfigRepository is intended to manage Config objects only. It contains methods for creating and updating invite codes, which are also implemented in InviteCodeRepository. Consolidating all invite code operations in InviteCodeRepository would reduce duplication and ease maintenance.
| await session.commit() | ||
| return new_user | ||
| # Use manual session management instead of transaction context manager | ||
| async for session in get_session(): |
There was a problem hiding this comment.
issue (complexity): Consider encapsulating the repeated session management logic in a reusable async context manager to reduce nesting and code duplication in transaction handling.
Consider encapsulating repeated session management logic in a reusable async context manager. This would reduce nesting and duplication of try/except blocks while keeping functionality intact. For example, you can create a helper like:
from contextlib import asynccontextmanager
@asynccontextmanager
async def transaction_context(get_session_fn):
async for session in get_session_fn():
try:
yield session
await session.commit()
except Exception:
await session.rollback()
raiseThen in your methods, refactor to use the context manager:
async def emby_create_user(self, telegram_id: int, username: str, password: str) -> User:
# ... other logic ...
async with transaction_context(get_session) as session:
if not user.enable_register and emby_config.register_public_user > 0:
emby_config.register_public_user -= 1
emby_config.total_register_user += 1
await ConfigRepository.update_config(
emby_config.id,
register_public_user=emby_config.register_public_user,
total_register_user=emby_config.total_register_user,
)
new_user = await self._emby_create_user(telegram_id, username, password)
# Persist new user and updated config if required
return new_userApply similar refactoring to the redeem_code method. This reduces nesting and centralizes the transaction handling.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Major: Refactor ORM Layer and Improve Docker Configuration
This PR makes significant changes to enhance our codebase in two key areas:
Changes
refactor(database): replace py_tools dependencies with native SQLAlchemy implementation
chore: remove requirements.txt that is replaced by pyproject.toml and uv.lock
fix(docker): update base image from alpine to slim for better compatibility
This change addresses potential compatibility issues with Alpine Linux
while maintaining a relatively small image footprint with the slim variant.
feat(docker): improve container build with proper Python dependency support
This change resolves compilation errors with tgcrypto and other Python packages
requiring C extensions, while keeping the Docker image as lightweight as possible.
Impact
Testing Done
好的，这是对pull request总结的中文翻译：
Sourcery 总结
重构 ORM 层以使用原生 SQLAlchemy，通过移除对自定义
py_tools库的依赖，提高可维护性和开发者体验。 此外，改进了 Docker 配置，以增强构建可靠性并解决依赖问题，确保正确处理 C 扩展并保持轻量级镜像。增强功能：
py_tools的自定义 ORM 实现替换为原生 SQLAlchemy 实现，包括用于数据访问的存储库类。get_session生成器的会话管理。app.py移动到模型文件来增强代码组织。测试：
Original summary in English
Summary by Sourcery
Refactors the ORM layer to use native SQLAlchemy, improving maintainability and developer experience by removing the dependency on the custom
py_toolslibrary. Also, improves the Docker configuration to enhance build reliability and resolve dependency issues, ensuring proper handling of C extensions and maintaining a lightweight image.Enhancements:
py_toolswith a native SQLAlchemy implementation, including repository classes for data access.get_sessiongenerator.app.pyto model files.Tests: